fix(debuginfo): Add some dwarf debuginfo recursion limits#1015
Conversation
Instructions and example for changelogPlease add an entry to Example: ## Unreleased
### Fixes
- Add some dwarf debuginfo recursion limits ([#1015](https://github.com/getsentry/symbolic/pull/1015))If none of the above apply, you can opt out of this check by adding |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| /// The maximum depth to recurse to when parsing inlined functions. | ||
| const MAX_PARSE_INLINEE_DEPTH: u32 = 256; |
There was a problem hiding this comment.
For this you could piggyback on the max_inline_depth option introduced in #1014.
18c09ee to
9921fec
Compare
loewenheim
left a comment
loewenheim
left a comment
There was a problem hiding this comment.
LGTM apart from the function removals. I think it's not uncommon in Rust for functions/methods to be defined both on a trait and on a type directly; this lets one use the function even if the trait isn't in scope. I would leave the existing functions in place if there isn't a strong reason to remove them.
| matches!(MachArchive::is_fat(data), Some(false)) | ||
| } | ||
|
|
||
| /// Tries to parse a MachO from the given slice. |
There was a problem hiding this comment.
I would leave this function in place. The removal is breaking for no real reason, IMO.
| Self::test(data) | ||
| } | ||
|
|
||
| fn parse_with_opts(data: &'d [u8], popts: ParseObjectOptions) -> Result<Self, Self::Error> { |
There was a problem hiding this comment.
Just for consistency:
| fn parse_with_opts(data: &'d [u8], popts: ParseObjectOptions) -> Result<Self, Self::Error> { | |
| fn parse_with_opts(data: &'d [u8], opts: ParseObjectOptions) -> Result<Self, Self::Error> { |
| } | ||
|
|
||
| /// Tries to parse a PE object from the given slice. | ||
| pub fn parse(data: &'data [u8]) -> Result<Self, PeError> { |
4c8c487 to
4b5e224
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4b5e224. Configure here.
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
| let mut entries = self.inner.unit.entries_raw(None)?; | ||
| let mut output = FunctionsOutput::with_seen_ranges(seen_ranges); | ||
| self.parse_functions(-1, &mut entries, &mut output)?; | ||
| self.parse_functions(-1, max_inline_depth, &mut entries, &mut output)?; |
There was a problem hiding this comment.
parse_function / parse_function_children lack depth limit for nested DW_TAG_subprogram
While the PR caps inlined-subroutine recursion with max_inline_depth, nested DW_TAG_subprogram entries are still traversed via mutual recursion between parse_function and parse_function_children with no depth bound. A crafted DWARF file with a chain of deeply nested subprograms causes an unbounded stack growth and an uncatchable stack-overflow abort.
Evidence
DwarfUnit::functions()at line 1359 initiates parsing by callingself.parse_functions(-1, max_inline_depth, &mut entries, &mut output)— the addedmax_inline_depthis only checked later on inlinees, not on subprograms.parse_functions(line 839) delegatesDW_TAG_subprogramentries toparse_function(line 855), passingremaining_inline_depthunchanged.parse_function(line 880) reads attributes then callsparse_function_children(line 979).parse_function_children(line 1019) recursively callsparse_functionfor nestedDW_TAG_subprogramchildren at greaternext_depth(line 1043) with no limit on nesting depth.- A malicious DWARF with a chain of nested subprograms creates an unbounded call stack (
parse_function→parse_function_children→parse_function→ …), while the sibling inlinee path was capped inparse_inlinee(line 1099).
Also found at 5 additional locations
symbolic-debuginfo/src/dwarf.rs:1760symbolic-debuginfo/src/pe.rs:260symbolic-debuginfo/src/dwarf.rs:586symbolic-debuginfo/src/dwarf.rs:1893symbolic-debuginfo/src/elf.rs:568
Identified by Warden · wrdn-dos-review · EVE-PMU
| max_decompressed_section_size: opts.max_decompressed_section_size, | ||
| max_inline_depth: opts.max_inline_depth, |
There was a problem hiding this comment.
ELF decompression size guard defaults to unlimited, allowing OOM abort on tiny input
max_decompressed_section_size defaults to None, which disables the size check and lets a tiny crafted ELF file declare a multi-GB compressed section, causing Vec::with_capacity(size) to abort the allocator.
Evidence
ParseObjectOptions::default()setsmax_decompressed_section_size: Default::default()which isNone(object.rs:152).- In
decompress_section,sizeis read from the attacker-controlled ELF compression header (elf.rs:610–634). - The check
size > self.max_decompressed_section_size.unwrap_or(usize::MAX)(elf.rs:637) becomessize > usize::MAXwhen the option isNone, so it never triggers. Vec::with_capacity(size)(elf.rs:643) is then called with the attacker-declared size, up tousize::MAX, causing an allocator abort on a tiny crafted input.- The same
ElfObjectstruct initialized on line 174 carries this ineffective limit intodecompress_section.
Also found at 2 additional locations
symbolic-debuginfo/src/elf.rs:359-359symbolic-debuginfo/src/object.rs:148-160
Identified by Warden · wrdn-dos-review · NRR-XKG
2 participants
Ref: INGEST-967